Conversation
`TopLevel#parser=` was calling `update_parser_of_file` with `absolute_name`, but `@files_hash` is keyed by `relative_name`. The lookup always failed, so `@text_files_hash` was never populated and `find_text_page` always returned nil.
These tests exercise the full pipeline: parsing source files, then resolving page URLs through the server's route method. They verify that text pages (.md, .rdoc), class pages, the index, and 404s all work correctly. The text page tests would have caught the update_parser_of_file key mismatch bug.
|
🚀 Preview deployment available at: https://c9681a98.rdoc-6cd.pages.dev (commit: de0e722) |
There was a problem hiding this comment.
Pull request overview
Fixes server-mode sidebar page links returning 404 by ensuring text pages are indexed using relative_name (matching @files_hash keys), and adds regression coverage for deferred parser assignment and server routing.
Changes:
- Pass
relative_name(notabsolute_name) fromRDoc::TopLevel#parser=intoRDoc::Store#update_parser_of_file. - Rename
update_parser_of_fileparameter for consistency with store keying. - Add unit + integration tests covering deferred parser assignment and server
routebehavior for pages/classes/index/404.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| lib/rdoc/code_object/top_level.rb | Updates parser assignment to notify the store using relative_name so text pages get indexed correctly. |
| lib/rdoc/store.rb | Aligns update_parser_of_file API/behavior with @files_hash keying (relative paths). |
| test/rdoc/rdoc_store_test.rb | Adds regression test for the deferred-parser path to ensure find_text_page works post-parser=. |
| test/rdoc/rdoc_server_test.rb | Adds routing tests to verify server-mode correctly serves pages/classes/index and returns 404 for missing pages. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Sets the parser of +relative_name+, unless it from a source code file. | ||
|
|
||
| def update_parser_of_file(absolute_name, parser) | ||
| if top_level = @files_hash[absolute_name] then | ||
| @text_files_hash[absolute_name] = top_level if top_level.text? | ||
| def update_parser_of_file(relative_name, parser) | ||
| if top_level = @files_hash[relative_name] then | ||
| @text_files_hash[relative_name] = top_level if top_level.text? |
There was a problem hiding this comment.
The method comment is inaccurate/grammatically incorrect: update_parser_of_file does not “set the parser”, it only updates @text_files_hash based on top_level.text? (and the sentence is missing “is”: “unless it is from ...”). Also, the parser parameter is unused; consider removing it or renaming to _parser to make the intent clear.
Summary
TopLevel#parser=was passingabsolute_nametoStore#update_parser_of_file, but@files_hashis keyed byrelative_name. The lookup always failed, so@text_files_hashwas never populated andfind_text_pagealways returned nil — making every page link in the sidebar 404.relative_nameinstead, and renamed the parameter inupdate_parser_of_filefor consistency.